Skip to content

Allow infinite ActiveState change listeners. #222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from

Conversation

eldh
Copy link

@eldh eldh commented Aug 22, 2014

Fixes a warning from event-emitter when using more than ten Links (or
other components using the ActiveState mixin) by allowing an unlimited number of listeners (as suggested here: http://nodejs.org/docs/latest/api/events.html#events_emitter_setmaxlisteners_n).

This is to try to fix issue #220.

Or would it be better to somehow group event listeners? Or is there some other way of solving this problem?

(Sorry if I'm doing this wrong, haven't done a pull request before.)

Fixes a warning from event-emitter when using more than ten Links (or
other components using the ActiveState mixin).
@sophiebits
Copy link
Contributor

I think this seems reasonable to me…

Fix for an error being thrown in Routes.js - updateMatchComponents.
@@ -281,7 +281,7 @@ function getRootMatch(matches) {

function updateMatchComponents(matches, refs) {
var i = 0, component;
while (component = refs[REF_NAME]) {
while ((component = refs[REF_NAME]) && (matches[i+1] !== undefined)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this all about? I remember debugging this code a while ago, can't remember why I was here though.

I'd like to know whats happening, not just prevent an error from happening.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh shit, that's not supposed to be there, I commited it to my fork after submitting the pull request. Didn't realize it would be added here (like I said, I'm new at this). Anyway, it's my proposed fix for #117.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing at a time :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted :)

@ryanflorence
Copy link
Member

closed by edbfd25

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants